feat(provider): add Passbolt provider support#1030
feat(provider): add Passbolt provider support#1030VDuchauffour wants to merge 1 commit intohelmfile:mainfrom
Conversation
VDuchauffour
requested review from
itscaro,
mumoshu,
xiaomudk and
yxxhero
as code owners
|
@VDuchauffour please fix DCO issue. |
VDuchauffour
force-pushed
the
feat/passbolt
branch
from
3fa934c to
b352900
Compare
|
@yxxhero it's done :) |
|
@VDuchauffour oh. please rebase your code. a lot of commits. |
VDuchauffour
force-pushed
the
feat/passbolt
branch
from
a504cf3 to
05d11fd
Compare
|
@yxxhero yep, sorry. It's done |
|
@VDuchauffour ci issue. |
Signed-off-by: Vincent Duchauffour <vincent.duchauffour@proton.me>
VDuchauffour
force-pushed
the
feat/passbolt
branch
from
05d11fd to
19eff99
Compare
|
@yxxhero fixed |
There was a problem hiding this comment.
Pull request overview
Adds a new passbolt backend to vals for retrieving secrets (including Passbolt v5.3+ custom fields), wires it into provider registries, and documents usage/config.
Changes:
- Introduce
pkg/providers/passboltprovider (GetString / GetStringMap) with sync.Once client initialization - Register
passboltprovider in runtime and stringprovider registries - Add Passbolt docs and new Go module dependencies + unit tests
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| vals.go | Registers the new passbolt provider in the runtime provider switch. |
| pkg/stringprovider/stringprovider.go | Adds passbolt to the string provider factory. |
| pkg/providers/passbolt/passbolt.go | Implements Passbolt provider logic (config resolution, login, field/custom-field extraction). |
| pkg/providers/passbolt/passbolt_test.go | Adds unit tests for config resolution and custom-field helpers/structures. |
| README.md | Documents Passbolt provider usage and configuration. |
| go.mod | Adds github.com/passbolt/go-passbolt dependency (+ indirect deps). |
| go.sum | Records checksums for newly added dependencies. |
| func (p *provider) GetString(key string) (string, error) { | ||
| if err := p.ensureClient(); err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| parts := strings.Split(key, "/") | ||
| if len(parts) == 0 || parts[0] == "" { | ||
| return "", fmt.Errorf("passbolt: key cannot be empty") | ||
| } |
There was a problem hiding this comment.
GetString/GetStringMap validate the key after ensureClient(). This makes invalid keys return unrelated client-init errors (e.g., missing GPG key) instead of "key cannot be empty", and it also prevents the key-parsing tests from exercising the parsing error paths. Parse/validate the key first (including extracting uuid / field), then call ensureClient() only once the key is known-valid, so callers get deterministic and accurate errors for malformed references.
| func (p *provider) GetStringMap(key string) (map[string]interface{}, error) { | ||
| if err := p.ensureClient(); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| parts := strings.Split(key, "/") | ||
| if len(parts) == 0 || parts[0] == "" { | ||
| return nil, fmt.Errorf("passbolt: key cannot be empty") | ||
| } |
There was a problem hiding this comment.
GetString/GetStringMap validate the key after ensureClient(). This makes invalid keys return unrelated client-init errors (e.g., missing GPG key) instead of "key cannot be empty", and it also prevents the key-parsing tests from exercising the parsing error paths. Parse/validate the key first (including extracting uuid / field), then call ensureClient() only once the key is known-valid, so callers get deterministic and accurate errors for malformed references.
| func (p *provider) getCustomFields(ctx context.Context, resourceID string) []customField { | ||
| resource, err := p.client.GetResource(ctx, resourceID) | ||
| if err != nil || resource.Metadata == "" { | ||
| return nil | ||
| } | ||
|
|
||
| rType, err := p.client.GetResourceType(ctx, resource.ResourceTypeID) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
|
|
||
| rawMeta, err := helper.GetResourceMetadata(ctx, p.client, resource, rType) | ||
| if err != nil { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
getCustomFields silently drops all errors by returning nil for any failure (resource fetch/type fetch/metadata decrypt/secret decrypt/unmarshal). This can lead to incorrect behavior where GetString(".../custom_fields/X") reports "not found" even though the provider actually failed to read/decrypt custom-field data. Consider returning (fields []customField, err error) (or at least logging the underlying error) and propagating it up through getResource/GetString/GetStringMap so failures are distinguishable from "no custom fields present".
| if err := json.Unmarshal([]byte(rawMeta), &metaRaw); err != nil || len(metaRaw.CustomFields) == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| secret, err := p.client.GetSecret(ctx, resource.ID) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
|
|
||
| rawSecret, err := p.client.DecryptSecretWithResourceID(resource.ID, secret.Data) | ||
| if err != nil { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
getCustomFields silently drops all errors by returning nil for any failure (resource fetch/type fetch/metadata decrypt/secret decrypt/unmarshal). This can lead to incorrect behavior where GetString(".../custom_fields/X") reports "not found" even though the provider actually failed to read/decrypt custom-field data. Consider returning (fields []customField, err error) (or at least logging the underlying error) and propagating it up through getResource/GetString/GetStringMap so failures are distinguishable from "no custom fields present".
| if err := json.Unmarshal([]byte(rawSecret), &secretData); err != nil { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
getCustomFields silently drops all errors by returning nil for any failure (resource fetch/type fetch/metadata decrypt/secret decrypt/unmarshal). This can lead to incorrect behavior where GetString(".../custom_fields/X") reports "not found" even though the provider actually failed to read/decrypt custom-field data. Consider returning (fields []customField, err error) (or at least logging the underlying error) and propagating it up through getResource/GetString/GetStringMap so failures are distinguishable from "no custom fields present".
| if err == nil { | ||
| t.Error("Expected error for empty key") | ||
| } | ||
|
|
||
| // The error will be from ensureClient or GetStringMap | ||
| // We just need to verify an error is returned |
There was a problem hiding this comment.
The test asserts err == nil twice, which is redundant and makes the intent harder to read. Remove one of the checks (and keep a single assertion with a single message) to simplify the test.
| if err == nil { | |
| t.Error("Expected error for empty key") | |
| } | |
| // The error will be from ensureClient or GetStringMap | |
| // We just need to verify an error is returned | |
| // The error will be from ensureClient or GetStringMap; we just need to verify an error is returned. |
| Parameters (override environment variables): | ||
|
|
||
| - `address`: Passbolt server URL | ||
| - `gpg_key_file`: Path to GPG private key file | ||
| - `passphrase`: GPG private key passphrase |
There was a problem hiding this comment.
The provider supports gpg_key as a configuration input (and it’s documented earlier in the PR description/env var list), but the README “Parameters” section omits the gpg_key query parameter. Add gpg_key here (and ideally also include it in the bracketed URI example) so users know they can pass key content directly via URI params.
Summary
Usage
ref+passbolt://RESOURCE_UUID#/password ref+passbolt://RESOURCE_UUID#/username ref+passbolt://RESOURCE_UUID#/custom_fields/ProjectID ref+passbolt://RESOURCE_UUID?address=https://passbolt.example.com#/passwordConfiguration
Supports both environment variables and URI query parameters:
Changes
Notes